Skip to content

C++: add isVla predicated to ArrayType #19298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 16, 2025
Merged

C++: add isVla predicated to ArrayType #19298

merged 4 commits into from
Apr 16, 2025

Conversation

IdrissRio
Copy link
Contributor

@IdrissRio IdrissRio commented Apr 14, 2025

This PR does the following:

  • Creates a new table, type_is_vla.
  • Defines a new predicate, isVla, in the ArrayType class.
  • Modifies the change notes and upgrade/downgrade scripts.

@IdrissRio IdrissRio added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Apr 14, 2025
@IdrissRio IdrissRio force-pushed the idrissrio/vla branch 3 times, most recently from 9d8403e to 3ed4a8d Compare April 14, 2025 08:07
@IdrissRio IdrissRio marked this pull request as ready for review April 14, 2025 10:04
@Copilot Copilot bot review requested due to automatic review settings April 14, 2025 10:04
@IdrissRio IdrissRio requested a review from a team as a code owner April 14, 2025 10:04
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new predicate, isVla(), in the ArrayType class to enable detection of variable-length arrays. Key changes include:

  • Creating a new table, type_is_vla.
  • Adding the isVla() predicate in ArrayType.
  • Updating change notes and upgrade/downgrade scripts.
Files not reviewed (4)
  • cpp/downgrades/fe54db1af25b6d8a1e25f6cbeae68baf615efc87/upgrade.properties: Language not supported
  • cpp/ql/lib/semmle/code/cpp/Type.qll: Language not supported
  • cpp/ql/lib/semmlecode.cpp.dbscheme: Language not supported
  • cpp/ql/lib/upgrades/e594389175c098d7225683d0fd8cefcc47d84bc1/upgrade.properties: Language not supported

@IdrissRio IdrissRio force-pushed the idrissrio/vla branch 3 times, most recently from 13eb715 to c39dc16 Compare April 14, 2025 12:34
Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment, which will unfortunately be a bit of annoying work, otherwise LGTM.

@@ -2178,6 +2178,8 @@ variable_vla(
int decl: @stmt_vla_decl ref
);

type_is_vla(unique int type_id: @type ref)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be a bit painful (because you need to re-do the upgrade scripts, and possibly also the stats file), but you should be able to do the following:

Suggested change
type_is_vla(unique int type_id: @type ref)
type_is_vla(unique int type_id: @derivedtype ref)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! 👍
I'll change that.

Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@IdrissRio IdrissRio merged commit 67bfe10 into main Apr 16, 2025
16 checks passed
@IdrissRio IdrissRio deleted the idrissrio/vla branch April 16, 2025 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants